feat(repo): Add eslint-plugin-tree-shaking #6916
Conversation
🦋 Changeset detectedLatest commit: 23dc0f2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughAdds a new local ESLint plugin package for tree-shaking analysis, integrates it into the root ESLint config for packages/react with a warning rule, and updates devDependencies. The plugin includes a recommended config, a comprehensive “no side effects in initialization” rule, supporting utilities, TypeScript config, README, and license. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant ESLint as ESLint CLI
participant Config as eslint.config.mjs
participant Plugin as @clerk/eslint-plugin-tree-shaking
participant Rule as no-side-effects-in-initialization
participant Utils as helpers/pure-functions/value
participant Code as packages/react/src/**/*
Dev->>ESLint: Run lint
ESLint->>Config: Load configuration
Config-->>ESLint: Extends Plugin recommended + rule warn
ESLint->>Plugin: Load plugin
Plugin-->>ESLint: Register rules/configs
loop For each file
ESLint->>Code: Parse to AST
ESLint->>Rule: Visit Program + nodes
Rule->>Utils: Resolve scopes, purity, values
alt Side-effects detected
Rule-->>ESLint: Report diagnostic(s)
else No side-effects
Rule-->>ESLint: No report
end
end
ESLint-->>Dev: Output warnings/errors
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (3)
🧰 Additional context used📓 Path-based instructions (6)**/*.{js,jsx,ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
packages/**/*.{ts,tsx,d.ts}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{ts,tsx}📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Files:
**/*.{js,ts,tsx,jsx}📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Files:
🧬 Code graph analysis (2)packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.ts (2)
packages/eslint-plugin-tree-shaking/src/utils/helpers.ts (2)
🪛 Biome (2.1.2)packages/eslint-plugin-tree-shaking/src/utils/helpers.ts[error] 31-31: Other switch clauses can erroneously access this declaration. The declaration is defined in this switch clause: Safe fix: Wrap the declaration in a block. (lint/correctness/noSwitchDeclarations) ⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🔇 Additional comments (21)
Comment |
@clerk/agent-toolkit
@clerk/astro
@clerk/backend
@clerk/chrome-extension
@clerk/clerk-js
@clerk/dev-cli
@clerk/elements
@clerk/clerk-expo
@clerk/expo-passkeys
@clerk/express
@clerk/fastify
@clerk/localizations
@clerk/nextjs
@clerk/nuxt
@clerk/clerk-react
@clerk/react-router
@clerk/remix
@clerk/shared
@clerk/tanstack-react-start
@clerk/testing
@clerk/themes
@clerk/types
@clerk/upgrade
@clerk/vue
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (1)
packages/eslint-plugin-tree-shaking/README.md (1)
14-18: Consider adding usage documentation.The README currently only contains branding and licensing information. Consider adding usage examples, configuration options, and available rules to help developers understand how to use this plugin.
Example sections to add:
- Installation instructions
- Configuration example
- Available rules and their options
- Magic comment annotations (if supported)
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (12)
eslint.config.mjs(2 hunks)package.json(1 hunks)packages/eslint-plugin-tree-shaking/LICENSE(1 hunks)packages/eslint-plugin-tree-shaking/README.md(1 hunks)packages/eslint-plugin-tree-shaking/package.json(1 hunks)packages/eslint-plugin-tree-shaking/src/index.ts(1 hunks)packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.ts(1 hunks)packages/eslint-plugin-tree-shaking/src/utils/helpers.ts(1 hunks)packages/eslint-plugin-tree-shaking/src/utils/pure-functions.ts(1 hunks)packages/eslint-plugin-tree-shaking/src/utils/value.ts(1 hunks)packages/eslint-plugin-tree-shaking/tsconfig.json(1 hunks)turbo.json(1 hunks)
🧰 Additional context used
📓 Path-based instructions (11)
**/*.{js,jsx,ts,tsx,json,css,scss,md,yaml,yml}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use Prettier for consistent code formatting
Files:
packages/eslint-plugin-tree-shaking/tsconfig.jsonpackages/eslint-plugin-tree-shaking/package.jsonpackages/eslint-plugin-tree-shaking/README.mdpackage.jsonturbo.jsonpackages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
packages/*/tsconfig.json
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Type checking must be performed with TypeScript and publint.
Files:
packages/eslint-plugin-tree-shaking/tsconfig.json
packages/*/package.json
📄 CodeRabbit inference engine (.cursor/rules/global.mdc)
All publishable packages should be placed under the packages/ directory
packages/*/package.json: All publishable packages must be located in the 'packages/' directory.
All packages must be published under the @clerk namespace on npm.
Semantic versioning must be used across all packages.
Files:
packages/eslint-plugin-tree-shaking/package.json
packages/**/README.md
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Maintain up-to-date README files for each package
Files:
packages/eslint-plugin-tree-shaking/README.md
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
**/*.{js,jsx,ts,tsx}: All code must pass ESLint checks with the project's configuration
Follow established naming conventions (PascalCase for components, camelCase for variables)
Maintain comprehensive JSDoc comments for public APIs
Use dynamic imports for optional features
All public APIs must be documented with JSDoc
Provide meaningful error messages to developers
Include error recovery suggestions where applicable
Log errors appropriately for debugging
Lazy load components and features when possible
Implement proper caching strategies
Use efficient data structures and algorithms
Profile and optimize critical paths
Validate all inputs and sanitize outputs
Implement proper logging with different levels
Files:
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
TypeScript is required for all packages
Files:
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
packages/**/*.{ts,tsx,d.ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Packages should export TypeScript types alongside runtime code
Files:
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use proper TypeScript error types
**/*.{ts,tsx}: Always define explicit return types for functions, especially public APIs
Use proper type annotations for variables and parameters where inference isn't clear
Avoidanytype - preferunknownwhen type is uncertain, then narrow with type guards
Useinterfacefor object shapes that might be extended
Usetypefor unions, primitives, and computed types
Preferreadonlyproperties for immutable data structures
Useprivatefor internal implementation details
Useprotectedfor inheritance hierarchies
Usepublicexplicitly for clarity in public APIs
Preferreadonlyfor properties that shouldn't change after construction
Prefer composition and interfaces over deep inheritance chains
Use mixins for shared behavior across unrelated classes
Implement dependency injection for loose coupling
Let TypeScript infer when types are obvious
Useconst assertionsfor literal types:as const
Usesatisfiesoperator for type checking without widening
Use mapped types for transforming object types
Use conditional types for type-level logic
Leverage template literal types for string manipulation
Use ES6 imports/exports consistently
Use default exports sparingly, prefer named exports
Use type-only imports:import type { ... } from ...
Noanytypes without justification
Proper error handling with typed errors
Consistent use ofreadonlyfor immutable data
Proper generic constraints
No unused type parameters
Proper use of utility types instead of manual type construction
Type-only imports where possible
Proper tree-shaking friendly exports
No circular dependencies
Efficient type computations (avoid deep recursion)
Files:
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
**/*.{js,ts,tsx,jsx}
📄 CodeRabbit inference engine (.cursor/rules/monorepo.mdc)
Support multiple Clerk environment variables (CLERK_, NEXT_PUBLIC_CLERK_, etc.) for configuration.
Files:
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.tspackages/eslint-plugin-tree-shaking/src/utils/pure-functions.tspackages/eslint-plugin-tree-shaking/src/utils/helpers.tspackages/eslint-plugin-tree-shaking/src/index.tspackages/eslint-plugin-tree-shaking/src/utils/value.ts
packages/**/index.{js,ts}
📄 CodeRabbit inference engine (.cursor/rules/development.mdc)
Use tree-shaking friendly exports
Files:
packages/eslint-plugin-tree-shaking/src/index.ts
**/index.ts
📄 CodeRabbit inference engine (.cursor/rules/react.mdc)
Use index.ts files for clean imports but avoid deep barrel exports
Avoid barrel files (index.ts re-exports) as they can cause circular dependencies
Files:
packages/eslint-plugin-tree-shaking/src/index.ts
🧬 Code graph analysis (3)
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.ts (2)
packages/eslint-plugin-tree-shaking/src/utils/value.ts (1)
Value(31-31)packages/eslint-plugin-tree-shaking/src/utils/helpers.ts (10)
getTreeShakingComments(128-128)isFunctionSideEffectFree(129-129)noEffects(132-132)getChildScopeForNodeIfExists(124-124)getLocalVariable(125-125)isLocalVariableAWhitelistedModule(126-126)isPureFunction(131-131)isFirstLetterUpperCase(130-130)getRootNode(127-127)hasPureNotation(133-133)
packages/eslint-plugin-tree-shaking/src/utils/helpers.ts (2)
eslint.config.mjs (2)
scope(73-73)variable(76-76)packages/eslint-plugin-tree-shaking/src/utils/pure-functions.ts (1)
pureFunctions(139-139)
packages/eslint-plugin-tree-shaking/src/index.ts (1)
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.ts (1)
noSideEffectsInInitialization(1062-1108)
🪛 Biome (2.1.2)
packages/eslint-plugin-tree-shaking/src/utils/helpers.ts
[error] 31-31: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🪛 markdownlint-cli2 (0.18.1)
packages/eslint-plugin-tree-shaking/README.md
5-5: Images should have alternate text (alt text)
(MD045, no-alt-text)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Build Packages
- GitHub Check: Formatting | Dedupe | Changeset
- GitHub Check: semgrep/ci
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: semgrep-cloud-platform/scan
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (8)
packages/eslint-plugin-tree-shaking/tsconfig.json (1)
1-10: LGTM! TypeScript configuration is appropriate.The tsconfig.json correctly uses NodeNext module resolution for ESM, enables strict type-checking, and targets ES2019 which is appropriate for modern Node.js environments.
package.json (1)
72-72: LGTM! Workspace dependency correctly added.The
workspace:*protocol is the correct way to reference the new ESLint plugin package in this pnpm monorepo.packages/eslint-plugin-tree-shaking/package.json (1)
1-20: LGTM! Package configuration is correct.The package.json properly configures the ESLint plugin with:
- Correct ESM setup (
type: "module")- Proper exports field for Node.js ESM resolution
- ESLint version (9.31.0) matching the root package.json
- Latest @types/estree version (1.0.8)
The
private: trueflag is appropriate for this internal tooling package.turbo.json (1)
105-105: LGTM! Lint task dependency correctly configured.Adding
@clerk/eslint-plugin-tree-shaking#buildto the lint task's dependencies ensures the ESLint plugin is built before linting runs. This is necessary since the ESLint config imports the plugin.packages/eslint-plugin-tree-shaking/LICENSE (1)
1-22: LGTM! License correctly attributes fork.The MIT License properly acknowledges both Clerk, Inc. as the fork maintainer (2022) and Lukas Taegert as the original author (2016). This is the correct approach for maintaining attribution in a fork.
packages/eslint-plugin-tree-shaking/src/utils/pure-functions.ts (1)
1-139: LGTM! Pure function map is comprehensive.The code correctly builds a lookup table of JavaScript built-ins and pure functions for tree-shaking analysis. The implementation is straightforward and covers the standard JavaScript API surface.
Note: The SIMD-related entries (lines 9-19) reference a deprecated JavaScript feature that was never standardized. While these won't match anything in modern code, keeping them doesn't cause any issues and maintains compatibility with the original plugin.
eslint.config.mjs (2)
4-4: LGTM! Plugin import added correctly.The tree-shaking plugin is properly imported using the workspace package reference.
410-417: LGTM! ESLint configuration for packages/react is appropriate.The configuration correctly:
- Targets the packages/react source files
- Applies the plugin's recommended configuration
- Sets the rule to 'warn' level for initial rollout
Using 'warn' instead of 'error' is a good practice when introducing a new linting rule, allowing developers to identify and fix issues without blocking their workflow.
| <a href="https://clerk.com?utm_source=github&utm_medium=clerk_types" target="_blank" rel="noopener noreferrer"> | ||
| <picture> | ||
| <source media="(prefers-color-scheme: dark)" srcset="https://images.clerk.com/static/logo-dark-mode-400x400.png"> | ||
| <img src="https://images.clerk.com/static/logo-light-mode-400x400.png" height="64"> |
There was a problem hiding this comment.
Add alt text to image for accessibility.
The Clerk logo image is missing alt text, which is important for screen readers and accessibility compliance.
Apply this diff to add appropriate alt text:
- <img src="https://images.clerk.com/static/logo-light-mode-400x400.png" height="64">
+ <img alt="Clerk Logo" src="https://images.clerk.com/static/logo-light-mode-400x400.png" height="64">📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <img src="https://images.clerk.com/static/logo-light-mode-400x400.png" height="64"> | |
| <img alt="Clerk Logo" src="https://images.clerk.com/static/logo-light-mode-400x400.png" height="64"> |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
5-5: Images should have alternate text (alt text)
(MD045, no-alt-text)
🤖 Prompt for AI Agents
packages/eslint-plugin-tree-shaking/README.md around line 5: the img tag for the
Clerk logo is missing alt text; update the tag to include an appropriate alt
attribute (e.g., alt="Clerk logo" or alt="Clerk logo (light mode)") so screen
readers can describe the image and improve accessibility.
| const leftValue = getAndReportLeft(); | ||
| if (!leftValue.hasValue) { | ||
| getAndReportRight(); | ||
| return leftValue; | ||
| } | ||
| if (leftValue.value) { | ||
| return leftValue; | ||
| } | ||
| return getAndReportRight(); | ||
| }, |
There was a problem hiding this comment.
Fix nullish coalescing evaluation
The ?? branch currently treats any falsy left operand (e.g., 0, '', false) as if it's null/undefined, so it wrongly evaluates the right-hand side and can emit false positives. Update the check to respect true nullish semantics.
- '??': (getAndReportLeft, getAndReportRight) => {
- const leftValue = getAndReportLeft();
- if (!leftValue.hasValue) {
- getAndReportRight();
- return leftValue;
- }
- if (leftValue.value) {
- return leftValue;
- }
- return getAndReportRight();
- },
+ '??': (getAndReportLeft, getAndReportRight) => {
+ const leftValue = getAndReportLeft();
+ if (!leftValue.hasValue) {
+ getAndReportRight();
+ return leftValue;
+ }
+ if (leftValue.value !== null && leftValue.value !== undefined) {
+ return leftValue;
+ }
+ return getAndReportRight();
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const leftValue = getAndReportLeft(); | |
| if (!leftValue.hasValue) { | |
| getAndReportRight(); | |
| return leftValue; | |
| } | |
| if (leftValue.value) { | |
| return leftValue; | |
| } | |
| return getAndReportRight(); | |
| }, | |
| '??': (getAndReportLeft, getAndReportRight) => { | |
| const leftValue = getAndReportLeft(); | |
| if (!leftValue.hasValue) { | |
| getAndReportRight(); | |
| return leftValue; | |
| } | |
| if (leftValue.value !== null && leftValue.value !== undefined) { | |
| return leftValue; | |
| } | |
| return getAndReportRight(); | |
| }, |
🤖 Prompt for AI Agents
In
packages/eslint-plugin-tree-shaking/src/rules/no-side-effects-in-initialization.ts
around lines 182–191, the nullish-coalescing branch treats any falsy left
operand as nullish; change the condition so it only short-circuits when the left
side is present and not null/undefined. Concretely, keep the existing
early-return when !leftValue.hasValue, but replace the truthy check with a
nullish check (ensure leftValue.hasValue is true AND leftValue.value !== null &&
leftValue.value !== undefined) to return leftValue; otherwise call and return
getAndReportRight().
| switch (node.type) { | ||
| case 'MemberExpression': | ||
| if (node.computed || node.property.type !== 'Identifier') { | ||
| return null; | ||
| } | ||
| // eslint-disable-next-line no-case-declarations | ||
| const flattenedParent = flattenMemberExpressionIfPossible(node.object); | ||
| return flattenedParent && `${flattenedParent}.${node.property.name}`; | ||
| case 'Identifier': |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Wrap switch clause declaration in its own block
The const flattenedParent declaration inside the switch clause currently leaks into other clauses and trips lint (noSwitchDeclarations). Wrap the clause in braces.
- case 'MemberExpression':
+ case 'MemberExpression': {
if (node.computed || node.property.type !== 'Identifier') {
return null;
}
// eslint-disable-next-line no-case-declarations
const flattenedParent = flattenMemberExpressionIfPossible(node.object);
return flattenedParent && `${flattenedParent}.${node.property.name}`;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| switch (node.type) { | |
| case 'MemberExpression': | |
| if (node.computed || node.property.type !== 'Identifier') { | |
| return null; | |
| } | |
| // eslint-disable-next-line no-case-declarations | |
| const flattenedParent = flattenMemberExpressionIfPossible(node.object); | |
| return flattenedParent && `${flattenedParent}.${node.property.name}`; | |
| case 'Identifier': | |
| switch (node.type) { | |
| case 'MemberExpression': { | |
| if (node.computed || node.property.type !== 'Identifier') { | |
| return null; | |
| } | |
| // eslint-disable-next-line no-case-declarations | |
| const flattenedParent = flattenMemberExpressionIfPossible(node.object); | |
| return flattenedParent && `${flattenedParent}.${node.property.name}`; | |
| } | |
| case 'Identifier': |
🧰 Tools
🪛 Biome (2.1.2)
[error] 31-31: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
🤖 Prompt for AI Agents
In packages/eslint-plugin-tree-shaking/src/utils/helpers.ts around lines 25 to
33, the const flattenedParent declared inside the 'MemberExpression' case is
leaking into other switch clauses and triggers the noSwitchDeclarations lint
rule; wrap the entire case body in its own block by adding opening and closing
braces immediately after the case label, keep the existing computed/property
checks and the flattenedParent declaration inside that block, then ensure you
return as before and add a break or rely on return to exit the case so the
switch behavior remains unchanged.
| const flattenedExpression = flattenMemberExpressionIfPossible(node); | ||
| if (context.options.length > 0) { | ||
| if ( | ||
| context.options[0].noSideEffectsWhenCalled.find( | ||
| (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression, | ||
| ) | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
| return flattenedExpression && pureFunctions[flattenedExpression]; | ||
| }; |
There was a problem hiding this comment.
Guard optional rule options before iterating
context.options[0].noSideEffectsWhenCalled is optional per the schema. When the rule is configured with {} (valid), this property is undefined, causing a crash at runtime. Please default it to an empty array before calling .find.
- if (context.options.length > 0) {
- if (
- context.options[0].noSideEffectsWhenCalled.find(
- (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression,
- )
- ) {
- return true;
- }
- }
+ const whitelist = context.options[0]?.noSideEffectsWhenCalled ?? [];
+ if (
+ whitelist.some(
+ (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression,
+ )
+ ) {
+ return true;
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const flattenedExpression = flattenMemberExpressionIfPossible(node); | |
| if (context.options.length > 0) { | |
| if ( | |
| context.options[0].noSideEffectsWhenCalled.find( | |
| (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression, | |
| ) | |
| ) { | |
| return true; | |
| } | |
| } | |
| return flattenedExpression && pureFunctions[flattenedExpression]; | |
| }; | |
| const flattenedExpression = flattenMemberExpressionIfPossible(node); | |
| - if (context.options.length > 0) { | |
| - if ( | |
| - context.options[0].noSideEffectsWhenCalled.find( | |
| - (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression, | |
| - ) | |
| - ) { | |
| - return true; | |
| - } | |
| const whitelist = context.options[0]?.noSideEffectsWhenCalled ?? []; | |
| if ( | |
| whitelist.some( | |
| (whiteListedFunction: any) => whiteListedFunction.function === flattenedExpression, | |
| ) | |
| ) { | |
| return true; | |
| } | |
| return flattenedExpression && pureFunctions[flattenedExpression]; | |
| }; |
🤖 Prompt for AI Agents
In packages/eslint-plugin-tree-shaking/src/utils/helpers.ts around lines 55 to
66, the code assumes context.options[0].noSideEffectsWhenCalled is always an
array and calls .find on it, which will throw when the rule is configured with
an empty object; guard this by defaulting that property to an empty array (e.g.,
const whitelist = context.options[0]?.noSideEffectsWhenCalled || []; then use
whitelist.find(...)) so the .find call never runs on undefined and the existing
logic remains unchanged.
| const isFunctionSideEffectFree = ( | ||
| functionName: string | undefined, | ||
| moduleName: any, | ||
| contextOptions: Rule.RuleContext['options'], | ||
| ) => { | ||
| if (contextOptions.length === 0) { | ||
| return false; | ||
| } | ||
|
|
||
| for (const whiteListedFunction of contextOptions[0].noSideEffectsWhenCalled) { | ||
| if ( | ||
| (whiteListedFunction.module === moduleName || | ||
| (whiteListedFunction.module === '#local' && moduleName[0] === '.')) && | ||
| (whiteListedFunction.functions === '*' || whiteListedFunction.functions.includes(functionName)) | ||
| ) { | ||
| return true; | ||
| } | ||
| } | ||
| return false; | ||
| }; |
There was a problem hiding this comment.
Avoid iterating undefined option arrays
Likewise here, contextOptions[0].noSideEffectsWhenCalled may be undefined, so iterating over it throws. Default the array before looping.
- if (contextOptions.length === 0) {
- return false;
- }
-
- for (const whiteListedFunction of contextOptions[0].noSideEffectsWhenCalled) {
+ if (contextOptions.length === 0) {
+ return false;
+ }
+
+ const whitelist = contextOptions[0]?.noSideEffectsWhenCalled ?? [];
+ if (whitelist.length === 0) {
+ return false;
+ }
+
+ for (const whiteListedFunction of whitelist) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const isFunctionSideEffectFree = ( | |
| functionName: string | undefined, | |
| moduleName: any, | |
| contextOptions: Rule.RuleContext['options'], | |
| ) => { | |
| if (contextOptions.length === 0) { | |
| return false; | |
| } | |
| for (const whiteListedFunction of contextOptions[0].noSideEffectsWhenCalled) { | |
| if ( | |
| (whiteListedFunction.module === moduleName || | |
| (whiteListedFunction.module === '#local' && moduleName[0] === '.')) && | |
| (whiteListedFunction.functions === '*' || whiteListedFunction.functions.includes(functionName)) | |
| ) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| }; | |
| const isFunctionSideEffectFree = ( | |
| functionName: string | undefined, | |
| moduleName: any, | |
| contextOptions: Rule.RuleContext['options'], | |
| ) => { | |
| if (contextOptions.length === 0) { | |
| return false; | |
| } | |
| const whitelist = contextOptions[0]?.noSideEffectsWhenCalled ?? []; | |
| if (whitelist.length === 0) { | |
| return false; | |
| } | |
| for (const whiteListedFunction of whitelist) { | |
| if ( | |
| (whiteListedFunction.module === moduleName || | |
| (whiteListedFunction.module === '#local' && moduleName[0] === '.')) && | |
| (whiteListedFunction.functions === '*' || whiteListedFunction.functions.includes(functionName)) | |
| ) { | |
| return true; | |
| } | |
| } | |
| return false; | |
| }; |
🤖 Prompt for AI Agents
In packages/eslint-plugin-tree-shaking/src/utils/helpers.ts around lines 85 to
104, contextOptions[0].noSideEffectsWhenCalled may be undefined and iterating it
will throw; default that property to an empty array before the for-loop (e.g.,
assign a local const to contextOptions[0].noSideEffectsWhenCalled || []) and
iterate that local variable so the loop safely runs even when the option is
omitted.
Description
This PR adds a forked copy of the
eslint-plugin-tree-shakingplugin with updates to support ESLint v9 and new JavaScript language features. The rule is enabled inwarnmode for@clerk/clerk-react.Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change
Summary by CodeRabbit
Chores
Documentation